implemented JSON distributed trace converter #335
Conversation
|
Looks like we're getting errors on the 1.15.x test run: |
v3/go.mod
Outdated
| github.com/golang/protobuf v1.4.3 | ||
| google.golang.org/grpc v1.39.0 |
There was a problem hiding this comment.
If we're requiring new versions of these libraries, will this affect our customers in any way?
There was a problem hiding this comment.
I think we can move the version back. That was automatically bumped by my go runtime, not because I purposefully upgraded it.
v3/newrelic/trace_context_test.go
Outdated
| {"{}", http.Header{}, false}, | ||
| {" invalid ", http.Header{}, true}, | ||
| {`"foo"`, http.Header{}, true}, | ||
| {`{"foo": "bar"}`, map[string][]string{ |
There was a problem hiding this comment.
Why is this map[string][]string rather than just another http.Header? I've changed them back to http.Header and all seems to work fine.
There was a problem hiding this comment.
http.Header is more correct to use, even though they're (currently) equivalent.
v3/newrelic/trace_context_test.go
Outdated
| "beta", | ||
| "gamma" | ||
| ] | ||
| }`, map[string][]string{ |
There was a problem hiding this comment.
Same here with http.Header vs. map[string][]string
v3/newrelic/transaction.go
Outdated
| // This is a convenience function provided for cases where you receive the trace header data | ||
| // already as a JSON string and want to avoid manually converting that to an http.Header. |
There was a problem hiding this comment.
I wonder if we want to add that this is likely when working with trace headers with different language agents? Maybe that would be too wordy. I just want to make sure somebody like Jo Ann would see this and use it.
There was a problem hiding this comment.
Not a bad idea. I don't mind leaning to more verbosity when it comes to documentation. You never know when that extra sentence or paragraph was just what someone was looking for.
Summary
When making calls to a transaction's
AcceptDistributedTraceHeaders()you are required to pass a validhttp.Headervalue holding the distributed trace headers you want the current transaction to use. However, sometimes this data arrives in a text form such as a JSON string.This requires the programmer to perform multiple steps to build up the
http.Headervalue, which can be prone to errors. I propose a convenience function that constructs this value in one step.Changes
This PR adds two functions.
DistributedTraceHeadersFromJson()converts a JSON representation of the trace headers to anhttp.Headerobject.AcceptDistributedTraceHeadersFromJson()does the above plus callsAcceptDistributedTraceHeaders()on the result for you.Details
Given a JSON string such as:
it will output the
http.Headervalue with three header keys (traceparent,tracestate, andnewrelic) with corresponding values as provided in the JSON string.It will also appropriately handle the case where multiple values are provided in the JSON string for a single key.
An error will be returned if it was unable to parse the JSON string or ran into other difficulties.